.changeset/lovely-cooks-add.md (1)

5-5: Call out new public utils surfaced by this PR.

This PR introduces public APIs in Bytes (Accumulator) and Memory (Slice). Consider mentioning them so downstream users see the new utilities in release notes.

Apply this diff to amend the body:

-`RLP`: Add library for Ethereum's Recursive Length Prefix encoding/decoding.
+`RLP`: Add library for Ethereum's Recursive Length Prefix encoding/decoding.
+
+Also introduces `Bytes.Accumulator` (a memory‑only bytes builder) and `Memory.Slice`
+utilities used by `RLP` and available for general use.

test/utils/Bytes.t.sol (2)

10-10: Narrow the using directive to avoid global pollution.

Limit extension scope to the types you actually use.

Apply:

-    using Bytes for *;
+    using Bytes for bytes;
+    using Bytes for bytes[];

---

227-237: Great golden‑path test for Accumulator.

Covers push/shift order and flatten result. Consider an explicit empty‑accumulator assertion as well.

Add alongside existing tests:

function testAccumulatorEmpty() public pure {
    Bytes.Accumulator memory acc = Bytes.accumulator();
    assertEq(acc.flatten(), hex"");
}

contracts/utils/Bytes.sol (2)

271-299: Return value on push/shift is unnecessary.

Since memory params are passed by reference for internal calls, returning Accumulator memory is redundant and invites accidental copies.

Apply:

-    function push(Accumulator memory self, bytes memory data) internal pure returns (Accumulator memory) {
+    function push(Accumulator memory self, bytes memory data) internal pure {
         // ...
-        return self;
     }
@@
-    function shift(Accumulator memory self, bytes memory data) internal pure returns (Accumulator memory) {
+    function shift(Accumulator memory self, bytes memory data) internal pure {
         // ...
-        return self;
     }

Note: update call sites only if they captured the return (current tests don’t).

---

141-161: concat() is clear and safe; optional micro‑opt.

You could preallocate and copy in one Yul loop, but current version favors readability and is fine.

contracts/utils/Memory.sol (1)

93-94: Clarify the out-of-bounds behavior in documentation.

The comment mentions that part of the return value will be out of bounds but should be ignored. This could lead to misuse where callers accidentally process invalid data.

Consider either:

1. Adding a bounds check to prevent reading beyond the slice
2. Updating the documentation to explicitly warn about undefined behavior

 /**
  * @dev Read a bytes32 buffer from a given Slice at a specific offset
  *
- * Note:If offset > length(slice) - 32, part of the return value will be out of bound and should be ignored.
+ * WARNING: If offset > length(slice) - 32, the return value will contain undefined data from memory
+ * beyond the slice bounds. Callers MUST ensure offset + 32 <= length(slice) or handle partial reads appropriately.
  */

contracts/utils/RLP.sol (3)

216-217: Consider removing or documenting the use case for readRawBytes.

The TODO comment questions the usefulness of this function. If there's no clear use case, consider removing it to reduce the API surface area.

Either document a specific use case or remove this function if it's not needed.

---

348-349: Replace TODO comments with proper custom errors.

Multiple locations use generic require statements with TODO comments indicating custom errors should be added for sanity checks.

Add specific custom errors for these validation cases:

+/// @dev Invalid RLP encoding: single byte string must be >= 0x80
+error RLPInvalidSingleByteString();
+
+/// @dev Invalid RLP encoding: length has leading zeros
+error RLPInvalidLeadingZeros();

 // Line 348-349
-require(bytes1(item.load(1)) >= bytes1(SHORT_OFFSET)); // TODO: custom error for sanity checks
+if (bytes1(item.load(1)) < bytes1(SHORT_OFFSET)) revert RLPInvalidSingleByteString();

 // Line 356
-require(bytes1(item.load(0)) != 0x00); // TODO: custom error for sanity checks  
+if (bytes1(item.load(0)) == 0x00) revert RLPInvalidLeadingZeros();

 // Line 376
-require(bytes1(item.load(0)) != 0x00);
+if (bytes1(item.load(0)) == 0x00) revert RLPInvalidLeadingZeros();

Also applies to: 356-357, 376-376

---

271-292: Add validation for currentOffset to prevent potential issues.

After the loop at line 285, there's no validation that currentOffset equals itemLength. This could hide malformed RLP data.

Add a validation check after the loop:

 uint256 itemCount;
 for (uint256 currentOffset = listOffset; currentOffset < itemLength; ++itemCount) {
     (uint256 elementOffset, uint256 elementLength, ) = _decodeLength(item.slice(currentOffset));
     list[itemCount] = item.slice(currentOffset, elementLength + elementOffset);
     currentOffset += elementOffset + elementLength;
 }
+
+// Ensure we consumed exactly the expected bytes
+require(currentOffset == itemLength, RLPContentLengthMismatch(itemLength, currentOffset));

 // Decrease the array size to match the actual item count.

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro